Skip to content

chore: check context in ngx.timer.* for exit process #1907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

rainingmaster
Copy link
Member

Since nginx has been confirmed that all timers have been cleaned up when exit worker is executed, all timers will no longer be executed in exit worker phase.
Reference https://github.com/nginx/nginx/blob/f02e2a734ef472f0dcf83ab2e8ce96d1acead8a5/src/os/unix/ngx_process_cycle.c#L715

README.markdown Outdated
@@ -8539,6 +8542,8 @@ table. It won't share the same `ngx.ctx` with the Lua handler creating the timer
If you need to pass data from the timer creator to the timer handler, please
use the extra parameters of `ngx.timer.at()`.

Since nginx has been confirmed that all timers have been cleaned up when exit worker is executed, all timers will no longer be executed in exit worker phase. Reference <https://github.com/nginx/nginx/blob/f02e2a734ef472f0dcf83ab2e8ce96d1acead8a5/src/os/unix/ngx_process_cycle.c#L715>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not allowed to create a timer (even a 0-delay timer) here since it runs after all timers have been processed.

better move this note to here?
https://github.com/openresty/lua-nginx-module/blob/e718d797cf28988251c329e247c7cd475d7e2bd9/README.markdown#exit_worker_by_lua_block

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we can keep the reference only in the code comments.

@rainingmaster rainingmaster force-pushed the exit_process_with_timer branch from e718d79 to 232469f Compare August 17, 2021 06:14
Copy link
Member

@doujiang24 doujiang24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One ci is failed, but it is not related to this change.
https://app.travis-ci.com/github/openresty/lua-nginx-module/jobs/531916997#L922

@doujiang24 doujiang24 merged commit 263bd0c into master Aug 18, 2021
@doujiang24 doujiang24 deleted the exit_process_with_timer branch August 18, 2021 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants